feat(sprintf): support Go-compatible %q for strings - #809
Conversation
Go's `%q` verb (strconv.Quote) is used by OPA's sprintf, but regorus currently bails on it unconditionally. Add support for `Value::String` arguments only: wrap in double quotes, escape `"` and `\`, use the short escapes for the common control characters (\a \b \f \n \r \t \v), fall back to \xNN/\uNNNN/\UNNNNNNNN for other non-printable characters, and leave printable (including non-ASCII) characters untouched. Unlike json.marshal, %q does not HTML-escape < > &. %q on non-string values (numbers, bools, etc.) still bails as before - Go's %q on those produces different, single-quoted output that is out of scope for this change. Verified byte-for-byte against real OPA output (strconv.Quote semantics) for quotes, backslashes, control characters, DEL, non-ASCII printable text, and the < > & non-escaping case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Follow-up for the complete format-spec parser and broader Go/OPA |
|
Would you prefer this change to be reviewed in two stages, or as one larger PR?
#810 is currently stacked on this PR, so its diff against |
|
Vitalii Tverdokhlib (@vitaliytv) Thank you for the contribution. Going to take a deeper look at this today/tomorrow. |
There was a problem hiding this comment.
🟡 Changes recommended
The new behavior lacks an RVM regression, so execution-path parity is not verified.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Go-compatible %q string formatting with Unicode-aware quoting and memory-limit propagation.
Changes:
- Adds Go Unicode printability tables and escaping.
- Integrates
%qwidth and precision handling. - Adds interpreter, unit, and memory-limit tests.
File summaries
| File | Description |
|---|---|
src/builtins/strings.rs |
Implements %q formatting. |
src/builtins/strings/go_is_print.rs |
Adds Unicode printability lookup. |
tests/interpreter/cases/builtins/strings/sprintf.yaml |
Tests interpreter behavior. |
tests/memory_limits.rs |
Tests memory-limit propagation. |
LICENSE |
Adds Go source attribution. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Hey Vitalii Tverdokhlib (@vitaliytv), Overall looks good to me. Can you also lock down the following as a test case: - note: "%q must escape Unicode line and paragraph separators"
data: {}
modules:
- |
package test
line_separator := sprintf("%q", ["a\u2028b"])
paragraph_separator := sprintf("%q", ["a\u2029b"])
query: data.test
want_result:
line_separator: "\"a\\u2028b\""
paragraph_separator: "\"a\\u2029b\"" |
done |
8f56e8f
into
microsoft:main
Summary
Add support for Go-syntax
%qformatting for Regorus string values. The implementation now matches Go 1.27.1 / OPA 1.20.2 for the supported string forms:\\xNN/\\uNNNN/\\UNNNNNNNNescapes;<,>, or&;sprintfparser (%Nq,%0Nq, and%.Nq);sprintfoutput buffer and propagates allocator memory-limit errors.Correctness changes
The initial implementation approximated Go's
strconv.IsPrintwith Rust character predicates. That differs for Unicode format characters, private-use characters, noncharacters, and unassigned code points. This revision ports the compact generated tables and lookup algorithm from Go 1.27.1'sstrconv/isprint.go(Unicode 17.0.0), so values such as U+00AD, U+200B, U+E000, U+FDD0, and U+0378 are escaped exactly like Go.Go's
%qapplies string precision before quoting and field width after quoting, both measured in Unicode code points. The supported Regorus forms now do the same, including Unicode input and zero padding.Verification
Compared directly with OPA 1.20.2 (built with Go 1.27.1) for regular strings, every escape class, printable Unicode, the Unicode edge cases above, width, zero padding, precision, and emoji width/precision.
Local checks:
cargo test --lockedcargo test --locked --no-default-features --libcargo check --locked --no-default-featurescargo xtask test-no-std%qcargo clippy --locked --lib --all-features -- -D warningsScope and full Go
%qThis PR intentionally handles
Value::Stringonly and preserves the repository's existing partialsprintfparser. A complete implementation of Go's%qwould also require:%8.3q);+for ASCII-only quoting,#for backquoted strings when allowed, and-for right padding;*,[n]);OPA adds another compatibility layer before calling
fmt.Sprintf: strings remain Go strings, fitting integers become Go integers, while booleans, nulls, arrays, objects, and sets are converted to their OPA string representation. Reproducing all of that is broader than a quoting helper and should be implemented with a proper format-spec parser and explicit OPA value-conversion rules rather than incrementally extending this string-only branch.